Repository navigation
feat(lsp): Phase 1 — bounded ingress, cancellation ownership, client gating - #3976
rossirpaulo wants to merge 2 commits into
Conversation
…gating Rust (stacked on the Phase 0 core): - IngressScheduler (transport-neutral admission/lifecycle/cancellation/ budget state machine) relocated into baml_lsp_server, with fixes: forced cancellation of protected methods can no longer be downgraded; permanent oversize is contained per-message (-32803) instead of closing the session; oversized $/cancelRequest no longer livelocks the control lane; pipelined `initialized` is admitted FIFO and accepted at dispatch. - New LspRuntime: stdio and browser LSP sessions dispatch through one scheduler; cancellation drained on the transport thread claims responses while handlers run ($/cancelRequest -> -32800); browser takeover revokes the old session's sink and replays document overlays; bounded outbound budgets with serialize-once shared frames. - Bounded stdio framing: 16 KiB header cap; oversized bodies discarded in chunks with a recoverable typed error; abnormal exit is nonzero. - bex_project kept minimal: LspError::RequestCanceled (-32800) at the one serialization boundary (I7); dispatch-scoped cancellation token checked at handler entry and after the source guard (never Salsa unwinding); connection-scoped LSP sessions (fresh encoding negotiation + workspace roots per connection, shared project registry). TypeScript: - D6 playground run-gating on is_bex_current: Run/test/preview disabled with a "Preparing current build..." state during rebuild windows; projectNotReady rejections render as that transient state and clear on the next current ProjectUpdate (no protocol version bump). - WebSocketRuntimePort reconnect correctness: fail-closed hello, bounded first-connect queue, session-scoped commands dropped on disconnect with synthesized rejections, desired-runtime lease resent after every hello. - Track A: projectRoots.ts canonical multi-root ownership + routing in the VS Code extension; closing the last document no longer stops the owner server; aggregate status preserves startup failures. Verification: baml_lsp_server 67/67, bex_project 45/45 (+1 pre-existing ignored), clippy -D warnings clean, wasm check clean; playground 125/125, extension 13/13, webview 19/19, all typechecks green. Harness (400 fns): $/cancelRequest now answers -32800; typing inlay p50 2.0 ms, p95 51.7 ms, zero errors, zero unanswered requests. Co-Authored-By: Claude Fable 5 <[email protected]> Claude-Session: https://claude.ai/code/session_01VEMxti5WQ8bCgJUuX9AQvo
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
⏭️ Performance benchmarks were skippedPerf benchmarks (CodSpeed) are opt-in on pull requests — they no longer run on every push. They always run automatically after merge to To run them on this PR, do any of the following, then push a commit (or re-run CI):
|
Binary size checks failed❌ 1 violations · ✅ 6 passed
Details & how to fixViolations:
Add/update baselines:
[artifacts.bridge_wasm]
file_bytes = 14760377
gzip_bytes = 4184019Generated by |
Drive-less file:///tmp URIs fail Url::to_file_path on Windows, so canonical_document_identity_str returned None and the test panicked on the Windows CI runner only. Co-Authored-By: Claude Fable 5 <[email protected]> Claude-Session: https://claude.ai/code/session_01VEMxti5WQ8bCgJUuX9AQvo
…ment + Phase 1 bounded ingress (BoundaryML#4000) Combines BoundaryML#3969 (Phase 0) and BoundaryML#3976 (Phase 1) into one PR against canary, with latest canary merged in (including the BoundaryML#3965 session-token removal and BoundaryML#3973 test-metadata work, both semantically reconciled). Supersedes both stacked PRs. Implements `docs/design/lsp-latency-and-mutex-fix.md` (committed in this PR) through Phase 1; Phase 2 (salsa snapshot reads) remains behind the doc's spike gate. ## Root cause The 0.14.0 debounced engine rebuild holds the project DB mutex across compiles while `try_lock_db` maps any contention to `-32001`; stale build candidates could install over newer edits, busy diagnostic reads looked empty, and playground/browser routing could leak stale session output. ## Phase 0 — correctness containment (from BoundaryML#3969) - Monotonic `SourceRevision`; owned compilation/diagnostic/engine candidates; **revision-conditional engine commit** through the same gate as source mutations — stale installs impossible by construction (rendezvous-tested at the exact commit boundary). - `DiagnosticsFence` judging staleness against the **authoritative revision**: a superseded rebuild's late diagnostics or a candidate racing `mark_dirty` can never regress newer markers. - Bounded request-lane waits (1s) mapping to typed errors — `ContentModified`/`RequestFailed`; **`-32001` is never emitted**. Busy is never conflated with empty; poison is terminal. - Salsa-memoized `file_annotations`/`semantic_tokens`; 150/300 ms debounced tails; single-flight rebuilds; UTF-8/UTF-16 `PositionCodec` (C1). ## Phase 1 — bounded ingress, cancellation, client gating (from BoundaryML#3976) - **IngressScheduler + LspRuntime** in `baml_lsp_server`: stdio and browser LSP sessions through one bounded scheduler (per-class byte/item budgets, lifecycle barriers, response ownership); `$/cancelRequest` handled on the transport thread — queued requests answer **`-32800`**, running cancellable requests observe a dispatch-scoped token (entry + post-source-guard safe points; never salsa unwinding); browser takeover tombstones the old session's sink and replays overlays. - Hardened framing/output: 16 KiB header cap; oversized messages/responses degrade **per-message** (`-32803`) instead of killing the session/process; serialize-once shared outbound frames with accurate budget accounting; connection-scoped sessions (per-connection encoding negotiation). - **Playground run-gating (D6)**: during rebuild windows (`is_bex_current == false` or a `projectNotReady` rejection) Run/test/preview disable with "Preparing current build…" and re-enable on the next current update — no protocol version bump. Closes the fail-closed D7 UX window Phase 0 introduces. - **Reconnect correctness**: fail-closed hello; bounded first-connect queue; session-scoped commands never replay into a new session (synthesized rejections); the desired-runtime lease survives reconnects. - **Track A**: canonical multi-root ownership (`projectRoots.ts`) + routing; closing the last `.baml` document no longer stops the owner server; aggregate status preserves startup failures. ## Measured (same harness/machine, 400 generated functions) | Metric | 0.14.1 (regressed) | this PR | |---|---:|---:| | Typing inlayHint p50 / p95 | 313.6 / 379.4 ms | **~2 / ~52 ms** | | Post-pause `-32001` failures | 46–57 per feature/window | **0** | | Unanswered requests | 144–160 | **0** | | Rebuild-window inlay/hover/completion | mass failures | 218/218 each | | Explicit `$/cancelRequest` | rejected/ignored | **`-32800`** | | Refresh storms per 10-edit burst | — | 0 | ## Deliberately not included - **Phase 2 salsa snapshots** (removes the residual post-pause hitch on very large projects): gated by the design doc's own spike-and-signoff decision — `panic=abort` release profile vs salsa's unwinding cancellation must be resolved first. - **D5 demand-gating server side**: client seam ships dormant (typed + tested, nothing sends it). - The alternate implementation's five-lock core rewrite (redundant with Phase 0's invariants), playground protocol-v3 lockstep (compat hazard; equivalent wire-compatible fixes included), and stdlib `testing.*` API additions (unauthorized surface). ## Validation - `bex_project` 45/45 (+1 pre-existing ignored) incl. commit-boundary rendezvous, fence, bounded-wait classification; `baml_lsp_server` 64/64 (22 scheduler, 9 runtime, framing/budget); workspace `cargo check --lib` clean; clippy `-D warnings` clean; wasm check clean. - TS: playground 125/125, extension 13/13, webview 19/19 (incl. gating), all typechecks. - Latency harness re-run on this exact merged head (numbers above). 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01VEMxti5WQ8bCgJUuX9AQvo <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Improved LSP responsiveness and stability with bounded messaging, cancellation handling, session recovery, and safer browser takeover behavior. * Added negotiated UTF-8/UTF-16 position handling for more accurate diagnostics, ranges, formatting, and semantic highlighting. * Added project-aware VS Code routing for nested projects, symlinks, and changing project markers. * Added runtime leasing support for playground connections. * **Bug Fixes** * Prevented runs and tests from starting while a project build is stale or unavailable, with clearer preparation feedback. * Improved cancellation, error reporting, diagnostics updates, and run generation consistency. * **Documentation** * Expanded contributor and Rust workspace setup, testing, formatting, and debugging guidance. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: Claude Fable 5 <[email protected]>
Stacked on #3969 (Phase 0). Do not merge before it; GitHub will retarget this to
canarywhen #3969 lands. Together the two PRs deliver the LSP latency/mutex design (docs/design/lsp-latency-and-mutex-fix.md) through Phase 1; Phase 2 (salsa snapshot reads) remains behind the design doc's spike gate.Summary
Rust — bounded ingress and cancellation (Track B2/D2/D3)
baml_lsp_serverwhere transport scheduling belongs): transport-neutral admission classes, lifecycle barriers, per-class byte/item budgets, response-ownership tokens, didChange tail-coalescing, browser takeover. Ported with fixes for every defect verified in review:NonCancellableWhenRunningoninitialize/shutdown/executeCommandcan no longer be downgraded by a caller policy (stronger_cancellation);-32803, notifications drop with a warn; the session and process stay alive;$/cancelRequestno longer livelocks the control lane;initializedduringInitializeRespondingis admitted FIFO and accepted at dispatch (found via the latency harness; the reference dropped it, breaking pipelining clients)./api/lspsessions dispatch through one scheduler behind a process-owned mutex;$/cancelRequestis drained on the transport thread so it can claim a response while a handler runs — queued requests answer-32800immediately, running cancellable requests observe a dispatch-scoped token (checked at handler entry and immediately after the source guard; never connected to salsa unwinding). Browser takeover atomically revokes the old session's sink (tombstone), purges its pending responses, and replays document overlays.Arc<[u8]>frames with a shared budget charge (accounting equals actual memory); per-session outbound budgets;Lagged/saturation close only the affected session; responses exceeding the reservation are replaced with a per-request-32803, never a session kill.LspError::RequestCanceled→-32800at the single serialization boundary (I7); connection-scoped LSP sessions (fresh position-encoding negotiation and workspace roots per connection, shared project registry) — fixes the global once-only encoding negotiation flagged in review.TypeScript — client gating (D6, D2 subset, Track A)
is_bex_currentis false (or a run/preview was refused withprojectNotReady), Run/test/preview actions disable with a "Preparing current build…" status and re-enable on the next currentProjectUpdate. No protocol version bump; closes the fail-closed D7 UX window refactor(lsp): Phase 0 latency and mutex containment #3969 introduces. Stale "using last successful build" copy removed.projectRoots.ts(canonical/symlink-aware root resolution, nested projects, multi-workspace) + routing in the extension — excluding two review-verified reference bugs: closing the last.bamldocument no longer stops the owner server (kept the playground webview alive), and aggregate status preserveserrorwhen every client start fails.Known limits (deliberate)
ensureProjectRuntime/releaseProjectRuntimeclient plumbing is dormant: typed, unit-tested, nothing sends them yet — the seam for the demand-gating server work (D5).didOpenin the same instant asinitialized(no ordering gap) can still have that notification dropped at admission — contract-conformant per the design doc; VS Code never does this.pendingTestTargetdeep-link auto-run is converted to the preparing state during a rebuild window but not auto-retried.Validation
cargo test --lib -p baml_lsp_server67/67 (22 scheduler, 9 runtime, 7 framing/budget, plus existing) ·-p bex_project45/45 (+1 pre-existing ignored)cargo clippy -p baml_lsp_server -p bex_project --all-targets -- -D warningsclean ·cargo check --target wasm32-unknown-unknown -p bridge_wasmclean ·cargo fmt --checkclean/tmp/baml-inlay-repro, 400 fns, release build of this branch):$/cancelRequestnow answers-32800(Phase 0: completed normally); typing inlay p50 2.0 ms / p95 51.7 ms; zero request errors; zero unanswered; 10-edit burst → 0 refresh storms.🤖 Generated with Claude Code
https://claude.ai/code/session_01VEMxti5WQ8bCgJUuX9AQvo